Reading and F-Order Files in v2 - #95
Open
konstibob wants to merge 1 commit into
Open
Conversation
`.zarray`'s `order` was parsed into `ArrayMetadata.order` and then never read,
so `"order": "F"` was silently treated as row-major in both directions:
* Reading an F-order store written by zarr-python returned the elements of
each chunk transposed. A 3x4 int32 array holding [[0,1,2,3],[4,5,6,7],
[8,9,10,11]] read back as [0, 4, 8, 1, 5, 9, 2, 6, 10, 3, 7, 11].
* Writing was worse: `withOrder(Order.F)` put `"order": "F"` into `.zarray`
while laying the chunk out row-major, so zarr-java emitted stores that
every other implementation misreads. zarr-python read such a store as
[[0,3,6,9],[1,4,7,10],[2,5,8,11]]. Because the reader shared the bug, the
round trip through zarr-java itself returned the correct values and no
Java-only test could see the corruption.
Zarr v3 dropped `order` and expresses the same layout with the `transpose`
codec, so `order` is modelled the same way here: a new `FortranOrderCodec`
array->array codec reverses all axes ahead of the `BytesCodec`, whose row-major
serialization of the reversed view is exactly the column-major serialization of
the logical chunk. Reversing axes is its own inverse, so one operation serves
both directions. The codec is never serialized; `order` stays the on-disk
representation, and it is only inserted for rank > 1, where the two orders
actually differ.
`decode` materializes the result with `Array.copy()`. A permuted view answers
iterators and `get(int[])` correctly but keeps a column-major backing store, so
linear accessors such as `getInt(int)` would walk it in the wrong order — and
the single-full-chunk fast path in `core.Array.read` hands a decoded chunk
straight to the caller.
Testing uses zarr-python as the oracle, since a Java-only round trip cannot
detect a symmetric bug:
* `testdata/golden/` gains four fixtures written by zarr-python (1.3 kB in
total, no compressor, so the chunk files are raw element bytes), covering a
C/F pair, rank 3, and a chunk grid that leaves partial edge chunks.
`src/test/python-scripts/generate_v2_order_golden.py` regenerates them.
* `ZarrV2OrderTest` checks both directions against those fixtures offline,
with no Python at test time: reads must yield `arange`, and writing
`arange` must reproduce zarr-python's chunk bytes exactly. It also pins
down that C and F differ on disk (uncompressed and compressed), that the
two coincide for rank 1, that a single-full-chunk read is physically
contiguous, and that `order` survives `resize` and attribute updates.
7 of its 15 tests fail without this change.
* `ZarrPythonTests` gains `testReadOrderV2`/`testWriteOrderV2`, which drive
the installed zarr-python in both directions so that a divergence from the
committed fixtures is noticed after a zarr-python upgrade.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.zarray'sorderwas parsed intoArrayMetadata.orderand then never read, so"order": "F"was silently treated as row-major in both directions:Reading an F-order store written by zarr-python returned the elements of each chunk transposed. A 3x4 int32 array holding [[0,1,2,3],[4,5,6,7], [8,9,10,11]] read back as [0, 4, 8, 1, 5, 9, 2, 6, 10, 3, 7, 11].
Writing was worse:
withOrder(Order.F)put"order": "F"into.zarraywhile laying the chunk out row-major, so zarr-java emitted stores that every other implementation misreads. zarr-python read such a store as [[0,3,6,9],[1,4,7,10],[2,5,8,11]]. Because the reader shared the bug, the round trip through zarr-java itself returned the correct values and no Java-only test could see the corruption.